[client] Feature/lazy connection #3379

pappz · 2025-02-24T16:38:25Z

Describe your changes

With the lazy connection feature, the peer will connect to target peers on-demand. The trigger can be any IP traffic.

This feature can be enabled with the NB_ENABLE_EXPERIMENTAL_LAZY_CONN environment variable.

When the engine receives a network map, it binds a free UDP port for every remote peer, and the system configures WireGuard endpoints for these ports. When traffic appears on a UDP socket, the system removes this listener and starts the peer connection procedure immediately.

Key changes

Fix slow netbird status -d command
Move from engine.go file to conn_mgr.go the peer connection related code
Refactor the iface interface usage and moved interface file next to the engine code
Add new command line flag and UI option to enable feature
The peer.Conn struct is reusable after it has been closed.
Change connection states

Connection states

Idle: The peer is not attempting to establish a connection. This typically means it's in a lazy state or the remote peer is expired.

Connecting: The peer is actively trying to establish a connection. This occurs when the peer has entered an active state and is continuously attempting to reach the remote peer.

Connected: A successful peer-to-peer connection has been established and communication is active.

Issue ticket number and link

Checklist

Is it a bug fix
Is a typo/documentation fix
Is a feature enhancement
It is a refactor
Created tests that fail without the change (if possible)
Extended the README / documentation, if necessary

Fix thread handling

…top function

Co-authored-by: hakansa <[email protected]>

fix idle state managmenet Validate the threshold env variable If the remote peer initiates the connection, we just reset the inactivity listener, but do not start it. As a result, after a disconnection, our peer never switches back to the idle state. For every peer, we instantiate an inactivity listener. It is a reusable timer, but it must start on a go routine after it has been stopped. The key challenge is to ensure that we never start the timer multiple times and always clean it up properly when the peer is in the idle state or it has been set to the exclusion list.

lixmal

PR description doesn't match code anymore (env vars)

client/cmd/root.go

lixmal · 2025-05-15T09:10:01Z

client/internal/lazyconn/activity/listener.go

+			if d.isClosed {
+				d.peerCfg.Log.Debugf("exit from activity listener")
+			} else {
+				d.peerCfg.Log.Errorf("failed to read from activity listener: %s", err)


Should we not retry (continue) reading if there's an error != closed? Maybe with a ctx check in-between

With it, we risk a high load loop. If it fails to read from UDP once, what makes us think it will succeed the second time?

With it, we risk a high load loop.

True

If it fails to read from UDP once, what makes us think it will succeed the second time?

It could be a temporary error, e.g. OS ran out of file descriptors.
But yeah, in general this should be rare. Maybe we can add a comment here explaining the implication for the time being?

client/internal/lazyconn/activity/listener.go

client/internal/lazyconn/activity/manager.go

client/internal/lazyconn/activity/allocator.go

client/internal/engine.go

client/internal/lazyconn/activity/listen_ip.go

client/internal/lazyconn/activity/manager.go

Add an option to the management proto to enable or disable the lazy connection feature on the peer side. Command-line options and environment variables override the management configuration. - When the feature is enabled: Start the lazy connection manager and add all peers to the manager in the "active" st-ate. - When the feature is disabled: Stop the lazy connection manager and establish connections to all peers in the store. Fix deadlock The conn_mgr is protected by a mutex. However, because some internal functions are called via callbacks from the lazy connection manager, a deadlock can occur. The solution is to completely eliminate the callbacks.

Copilot

Pull Request Overview

This PR adds an experimental “lazy connection” feature allowing on-demand WireGuard peer connections and refactors related engine and stats handling.

Introduce ConnMgr to manage lazy vs. permanent connections based on traffic or feature flag
Integrate ConnMgr into the engine startup/stop flows and expose CLI/config flags
Refactor WG stats interfaces to return a map of stats and update parsing logic

Reviewed Changes

Copilot reviewed 60 out of 60 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
client/internal/lazyconn/activity/listener.go	UDP listener for on-demand connection trigger
client/internal/lazyconn/activity/listen_ip_linux.go	Linux build listen IP definition
client/internal/lazyconn/activity/listen_ip.go	Default listen IP for other platforms
client/internal/iface_common.go	Change `GetStats` to return `map[string]WGStats`
client/internal/engine_test.go	Update tests to initialize and start `ConnMgr`
client/internal/engine.go	Wire up feature flag, engine start/stop, and peer flows
client/internal/debug/debug.go	Add `LazyConnectionEnabled` to debug bundle output
client/internal/connect.go	Propagate lazy-connection flag into `EngineConfig`
client/internal/conn_mgr.go	New `ConnMgr` implementing lazy connection logic
client/internal/config.go	Read/apply `LazyConnectionEnabled` in config
client/iface/iface.go	Update `GetStats` signature to return map
client/iface/device/interface.go	Update `GetStats` signature to return map
client/iface/configurer/usp_test.go	Adapt tests for new stats parsing function
client/iface/configurer/usp.go	Refactor stats parsing into `parseTransfers` & helper funcs
client/iface/configurer/kernel_unix.go	Update stats method to return map of WGStats
client/cmd/up.go	Set lazy flag for `up` command
client/cmd/status.go	Extend status filter options to include `idle
client/cmd/root.go	Add `--enable-lazy-connection` CLI flag
client/internal/dns/wgiface_windows.go	Remove outdated `GetStats(peerKey)` from DNS iface
client/internal/dns/wgiface.go	Remove outdated `GetStats(peerKey)` from DNS iface

Comments suppressed due to low confidence (2)

client/internal/lazyconn/activity/listener.go:33

[nitpick] The error message has a grammatical issue: replace "failed to creating" with "failed to create".

return nil, fmt.Errorf("failed to creating activity listener: %v", err)

client/iface/configurer/usp.go:287

[nitpick] The local variable TxBytes begins with an uppercase letter, which is typically reserved for exported identifiers. Rename it to txBytes to follow Go naming conventions.

TxBytes, err := toBytes(key[1])

Copilot · 2025-05-20T08:57:37Z

client/internal/lazyconn/activity/listener.go

+	for {
+		n, remoteAddr, err := d.conn.ReadFromUDP(make([]byte, 1))


Allocating a new buffer on each ReadFromUDP call can be inefficient; consider reusing a single []byte buffer declared outside the loop.

Suggested change

for {

n, remoteAddr, err := d.conn.ReadFromUDP(make([]byte, 1))

buffer := make([]byte, 1) // Reusable buffer for reading packets

for {

n, remoteAddr, err := d.conn.ReadFromUDP(buffer)

client/internal/lazyconn/activity/listen_ip_linux.go

client/iface/configurer/usp.go

Copilot · 2025-05-20T08:57:39Z

client/internal/connect.go

@@ -481,7 +482,7 @@ func connectToSignal(ctx context.Context, wtConfig *mgmProto.NetbirdConfig, ourP
 	return signalClient, nil
 }

-// loginToManagement creates Management Services client, establishes a connection, logs-in and gets a global Netbird config (signal, turn, stun hosts, etc)
+// loginToManagement creates Management ServiceDependencies client, establishes a connection, logs-in and gets a global Netbird config (signal, turn, stun hosts, etc)


[nitpick] The comment refers to a "Management ServiceDependencies client", which is unclear. Consider restoring "Management Services client" or clarifying what "ServiceDependencies" refers to.

Suggested change

// loginToManagement creates Management ServiceDependencies client, establishes a connection, logs-in and gets a global Netbird config (signal, turn, stun hosts, etc)

// loginToManagement creates Management Services client, establishes a connection, logs-in and gets a global Netbird config (signal, turn, stun hosts, etc)

sonarqubecloud · 2025-05-20T11:50:49Z

Quality Gate passed

Issues
0 New issues
8 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.1% Duplication on New Code

See analysis details on SonarQube Cloud

pappz added 30 commits February 21, 2025 13:36

Initial concept

db278db

Merge branch 'main' into feature/lazy-connection

c8b031e

Fix logic

d6e1850

Add for engine

0457251

Create conn mgr

8542674

Add test for port allocator

309e825

Remove watcher

e2e1458

Handle env var

f898904

Add log lines

e6e070b

Add log lines

884d10c

Fix constructor of conn.go

9188fca

Fix slow netbird status -d issue

6ad66fe

Fix ip indication for lazy peers in status

a27227f

Fix close

fcca194

Fix context cancellation

4f668fd

Fix blocking Close

aa3cd20

Rename fake to listener

d0d37ba

Fix thread handling

Fix postponed remove endpoint config

4a5edc1

Fix deprecated event handling

4e33582

Fix deadlock

feb8355

Fix close channel creation

4c39ba3

Fix TestEngine_UpdateNetworkMap

f8a4cfb

Fix TestEngine_UpdateNetworkMap

b38ddd8

Replace os.setenv to t.setenv

3ff3bbe

Fix use mgr without start

a89a6c0

Fix unit test

04f18bf

Disable lazy connection in multiple peers test

0f1f023

Avoid panic in test

a3e7604

Fix udp address bind

f09d861

Fix minimum port range

dddb4bc

pappz and others added 5 commits May 5, 2025 10:46

Merge branch 'main' into feature/lazy-connection

e2ccccd

Fix nil pointer exception if the engine is not started yet and call s…

e503148

…top function

Fix exclude list handling (#3797)

7eb8e3b

Feature/lazy connection status filter fix (#3795)

8e64c9e

Co-authored-by: hakansa <[email protected]>

lixmal reviewed May 15, 2025

View reviewed changes

pappz added 8 commits May 15, 2025 12:53

Fix command line description text

f6ac268

Reduce the buffer size

0e7d6c3

Remove port allocator

176b824

Change log message

71d683d

Convert bool to atomic bool

cf36afe

Add err context

e6b4db8

Merge branch 'main' into feature/lazy-connection

98fc917

lixmal previously approved these changes May 15, 2025

View reviewed changes

Update minimum version

6469fcc

pappz dismissed lixmal’s stale review via 6469fcc May 19, 2025 07:17

Fix support test

6fbe25f

lixmal previously approved these changes May 20, 2025

View reviewed changes

Update minimum version

e021f77

pappz dismissed lixmal’s stale review via e021f77 May 20, 2025 08:49

lixmal previously approved these changes May 20, 2025

View reviewed changes

mlsmaycon requested a review from Copilot May 20, 2025 08:54

Copilot AI reviewed May 20, 2025

View reviewed changes

Remove nanosec precision from last handshake

535d9fb

pappz dismissed lixmal’s stale review via 535d9fb May 20, 2025 11:50

lixmal approved these changes May 20, 2025

View reviewed changes

pappz merged commit daa8380 into main May 21, 2025
35 checks passed

pappz deleted the feature/lazy-connection branch May 21, 2025 09:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

[client] Feature/lazy connection #3379

[client] Feature/lazy connection #3379

Uh oh!

pappz commented Feb 24, 2025 •

edited

Loading

Uh oh!

lixmal left a comment

Uh oh!

Uh oh!

lixmal May 15, 2025

Uh oh!

pappz May 15, 2025

Uh oh!

lixmal May 15, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI May 20, 2025

Uh oh!

Uh oh!

Uh oh!

Copilot AI May 20, 2025

Uh oh!

sonarqubecloud bot commented May 20, 2025

Uh oh!

Uh oh!

Uh oh!

		for {
		n, remoteAddr, err := d.conn.ReadFromUDP(make([]byte, 1))

-	for {
-		n, remoteAddr, err := d.conn.ReadFromUDP(make([]byte, 1))
+	buffer := make([]byte, 1) // Reusable buffer for reading packets
+	for {
+		n, remoteAddr, err := d.conn.ReadFromUDP(buffer)

	// loginToManagement creates Management ServiceDependencies client, establishes a connection, logs-in and gets a global Netbird config (signal, turn, stun hosts, etc)
	// loginToManagement creates Management Services client, establishes a connection, logs-in and gets a global Netbird config (signal, turn, stun hosts, etc)

Uh oh!

[client] Feature/lazy connection #3379

[client] Feature/lazy connection #3379

Uh oh!

Conversation

pappz commented Feb 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Describe your changes

Key changes

Connection states

Issue ticket number and link

Checklist

Uh oh!

lixmal left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lixmal May 15, 2025

Choose a reason for hiding this comment

Uh oh!

pappz May 15, 2025

Choose a reason for hiding this comment

Uh oh!

lixmal May 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI May 20, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI May 20, 2025

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented May 20, 2025

Quality Gate passed

Uh oh!

Uh oh!

Uh oh!

pappz commented Feb 24, 2025 •

edited

Loading

lixmal May 15, 2025 •

edited

Loading